feat: first-run auto-init, overall progress, completion celebration (#13, #15, #16)#20
feat: first-run auto-init, overall progress, completion celebration (#13, #15, #16)#20abhiksark wants to merge 3 commits into
Conversation
Closes #13. Running `pylings` (or watch/start/topics) in a directory without a workspace previously raised ManifestError and exited 2 — the #1 beginner trap. Now it auto-creates a workspace at ./pylings-workspace (reusing one if already present) and launches into it. - New ensure_workspace(root) in core/curriculum.py: returns root if it is already a workspace, reuses a prior ./pylings-workspace, else inits one. Returns (path, created_new). - cli.main() resolves the root via ensure_workspace before run_tui and prints where the workspace is when it differs from the requested root. - Tests: unit coverage for ensure_workspace (3 cases) and a headless CLI integration test (start with a bad topic returns before the TUI).
Closes #15. The progress bar previously showed only the current topic's completion. It now also shows overall progress across all exercises, so learners can see how far they are in the whole curriculum. - New pure format_progress(...) in widgets/progress.py renders both the topic and overall bars/counts; ProgressBar.update_progress delegates to it. - TrackScreen._render_state passes overall counts (len(state.completed) vs len(manifest.exercises)). - Tests cover the formatter incl. the zero-total edge case.
Closes #16. A per-topic completion message already existed, but finishing the entire curriculum had no payoff. When the final pending exercise across all topics is completed, the track screen now shows a celebration (count, a shareable line, and a GitHub-star nudge) instead of the plain topic-complete message. - celebration_message(total) builds the text; shown via OutputPanel.show_final. - Gated on next_pending(manifest.exercises, completed) is None, so it fires only at true 100%. Non-blocking: F4/Ctrl+Q still work. - Unit test covers the message content.
📝 WalkthroughWalkthroughThe PR implements three features from the 0.3.0 adoption roadmap: auto-initializing a workspace on first run (preventing ChangesAdoption & Motivation UX
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pylings/cli.py`:
- Around line 293-299: ensure_workspace can raise WorkspaceError but the current
code only converts ManifestError, so wrap the call to ensure_workspace in a
try/except that catches WorkspaceError (import WorkspaceError from
pylings.core.curriculum) and handle it gracefully: print a concise,
user-friendly message including the error details (e.g., "Workspace appears
invalid: {e}. Try removing or repairing the pylings-workspace or run init") and
exit with a non-zero status (sys.exit(1)); keep the existing created/root logic
for successful returns.
In `@pylings/core/curriculum.py`:
- Around line 96-100: The current call to init_workspace(default) can raise
WorkspaceError when root/pylings-workspace exists and is non-empty but lacks
info.toml; wrap the init_workspace(default) call in a try/except for
WorkspaceError and, if the directory already exists and contains files (i.e.,
default.exists() and any(default.iterdir())), treat it as an existing workspace
by returning default.resolve(), False; otherwise re-raise the WorkspaceError.
Reference init_workspace and WorkspaceError so you can locate and update the
error handling around that call.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c6d11b1b-5340-42ec-8299-2fce47491bda
📒 Files selected for processing (8)
pylings/cli.pypylings/core/curriculum.pypylings/screens/track.pypylings/widgets/progress.pytests/integration/test_cli_autoinit.pytests/unit/test_celebration.pytests/unit/test_progress.pytests/unit/test_workspace_autoinit.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (8)
tests/{unit,integration,tui}/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Tests should be organized in
tests/unit/,tests/integration/, andtests/tui/directories with fixtures intests/fixtures/
Files:
tests/unit/test_celebration.pytests/unit/test_workspace_autoinit.pytests/unit/test_progress.pytests/integration/test_cli_autoinit.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Use Python 3.11+ idioms and 4-space indentation
Prefer small, typed functions where practical
Files:
tests/unit/test_celebration.pypylings/screens/track.pytests/unit/test_workspace_autoinit.pypylings/widgets/progress.pypylings/cli.pypylings/core/curriculum.pytests/unit/test_progress.pytests/integration/test_cli_autoinit.py
tests/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
tests/**/*.py: Name test files astest_<behavior>.pyand test functions astest_<expected_behavior>
Use pytest for all tests; async tests are supported bypytest-asyncioin auto mode
Files:
tests/unit/test_celebration.pytests/unit/test_workspace_autoinit.pytests/unit/test_progress.pytests/integration/test_cli_autoinit.py
tests/unit/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Add unit tests for core behavior in
tests/unit/
Files:
tests/unit/test_celebration.pytests/unit/test_workspace_autoinit.pytests/unit/test_progress.py
pylings/{screens,widgets}/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Textual screens and widgets should live in
pylings/screens/andpylings/widgets/
Files:
pylings/screens/track.pypylings/widgets/progress.py
pylings/{cli.py,__main__.py}
📄 CodeRabbit inference engine (AGENTS.md)
CLI entry points should be in
pylings/cli.pyandpylings/__main__.py
Files:
pylings/cli.py
pylings/core/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Core exercise loading, workspace setup, state, reset, solutions, and runner logic should live in
pylings/core/
Files:
pylings/core/curriculum.py
tests/integration/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Add integration tests for CLI/workspace flows in
tests/integration/
Files:
tests/integration/test_cli_autoinit.py
🧠 Learnings (6)
📚 Learning: 2026-05-25T15:06:34.945Z
Learnt from: CR
Repo: abhiksark/pylings PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-05-25T15:06:34.945Z
Learning: Applies to tests/integration/**/*.py : Add integration tests for CLI/workspace flows in `tests/integration/`
Applied to files:
tests/unit/test_workspace_autoinit.pypylings/cli.pytests/integration/test_cli_autoinit.py
📚 Learning: 2026-05-25T15:06:34.945Z
Learnt from: CR
Repo: abhiksark/pylings PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-05-25T15:06:34.945Z
Learning: Applies to tests/unit/**/*.py : Add unit tests for core behavior in `tests/unit/`
Applied to files:
tests/unit/test_workspace_autoinit.pytests/unit/test_progress.py
📚 Learning: 2026-05-25T15:06:34.945Z
Learnt from: CR
Repo: abhiksark/pylings PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-05-25T15:06:34.945Z
Learning: Applies to pylings/core/**/*.py : Core exercise loading, workspace setup, state, reset, solutions, and runner logic should live in `pylings/core/`
Applied to files:
tests/unit/test_workspace_autoinit.pypylings/core/curriculum.py
📚 Learning: 2026-05-25T15:06:34.945Z
Learnt from: CR
Repo: abhiksark/pylings PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-05-25T15:06:34.945Z
Learning: Applies to pylings/{cli.py,__main__.py} : CLI entry points should be in `pylings/cli.py` and `pylings/__main__.py`
Applied to files:
pylings/cli.py
📚 Learning: 2026-05-25T15:06:34.945Z
Learnt from: CR
Repo: abhiksark/pylings PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-05-25T15:06:34.945Z
Learning: When changing curriculum, update `exercises/`, `checks/`, `solutions/`, and `info.toml`, then run relevant pytest files plus `pylings --root tests/fixtures/passing_curriculum verify`
Applied to files:
pylings/cli.pypylings/core/curriculum.py
📚 Learning: 2026-05-25T15:06:34.945Z
Learnt from: CR
Repo: abhiksark/pylings PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-05-25T15:06:34.945Z
Learning: Applies to tests/tui/**/*.py : Add TUI tests for Textual interactions in `tests/tui/`
Applied to files:
tests/unit/test_progress.py
🪛 Ruff (0.15.15)
tests/integration/test_cli_autoinit.py
[error] 9-9: subprocess call: check for execution of untrusted input
(S603)
🔇 Additional comments (6)
tests/unit/test_workspace_autoinit.py (1)
8-31: LGTM!tests/integration/test_cli_autoinit.py (1)
8-21: LGTM!pylings/widgets/progress.py (1)
6-39: LGTM!tests/unit/test_progress.py (1)
6-22: LGTM!pylings/screens/track.py (1)
22-30: LGTM!Also applies to: 193-202
tests/unit/test_celebration.py (1)
6-10: LGTM!
| from pylings.core.curriculum import ensure_workspace | ||
| root, created = ensure_workspace(args.root) | ||
| if root != args.root.expanduser().resolve(): | ||
| if created: | ||
| print(f"No pylings workspace found here; created one at {root}") | ||
| else: | ||
| print(f"Using existing pylings workspace at {root}") |
There was a problem hiding this comment.
Handle WorkspaceError from ensure_workspace gracefully.
ensure_workspace can raise WorkspaceError (e.g., a non-empty ./pylings-workspace without info.toml). The handler at lines 316-322 only converts ManifestError, so this surfaces as a raw traceback — the opposite of the friendly first-run behavior this PR targets.
🛡️ Proposed fix
if args.command in (None, "watch", "start", "topics"):
- from pylings.core.curriculum import ensure_workspace
- root, created = ensure_workspace(args.root)
+ from pylings.core.curriculum import WorkspaceError, ensure_workspace
+ try:
+ root, created = ensure_workspace(args.root)
+ except WorkspaceError as e:
+ sys.stderr.write(f"pylings: {e}\n")
+ return 2
if root != args.root.expanduser().resolve():🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pylings/cli.py` around lines 293 - 299, ensure_workspace can raise
WorkspaceError but the current code only converts ManifestError, so wrap the
call to ensure_workspace in a try/except that catches WorkspaceError (import
WorkspaceError from pylings.core.curriculum) and handle it gracefully: print a
concise, user-friendly message including the error details (e.g., "Workspace
appears invalid: {e}. Try removing or repairing the pylings-workspace or run
init") and exit with a non-zero status (sys.exit(1)); keep the existing
created/root logic for successful returns.
| default = root / DEFAULT_WORKSPACE_DIRNAME | ||
| if (default / "info.toml").exists(): | ||
| return default.resolve(), False | ||
|
|
||
| return init_workspace(default), True |
There was a problem hiding this comment.
Edge case: non-empty pylings-workspace without info.toml raises WorkspaceError.
If root/pylings-workspace already exists and is non-empty but lacks info.toml (e.g., a partial/aborted init or a user-created dir), init_workspace(default) raises WorkspaceError ("already exists and is not empty"). This propagates uncaught through cli.main (which only handles ManifestError), reintroducing a crash on the very first-run path this PR aims to smooth. See the related comment in cli.py.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pylings/core/curriculum.py` around lines 96 - 100, The current call to
init_workspace(default) can raise WorkspaceError when root/pylings-workspace
exists and is non-empty but lacks info.toml; wrap the init_workspace(default)
call in a try/except for WorkspaceError and, if the directory already exists and
contains files (i.e., default.exists() and any(default.iterdir())), treat it as
an existing workspace by returning default.resolve(), False; otherwise re-raise
the WorkspaceError. Reference init_workspace and WorkspaceError so you can
locate and update the error handling around that call.
|
Superseded by the v0.4.0 |
1 participant
Closes #13
Closes #15
Closes #16
Part of the 0.3.0 adoption roadmap — First-run UX & motivation. Built test-first (TDD).
#13 — auto-init a workspace when launching with none
Running
pylings(orwatch/start/topics) in a directory without aworkspace previously raised
ManifestErrorand exited 2 — the #1 beginnertrap. It now auto-creates
./pylings-workspace(reusing an existing one) andlaunches into it.
ensure_workspace(root) -> (path, created)incore/curriculum.py.cli.main()resolves the root via it beforerun_tui, printing where theworkspace is when it differs from the requested root.
#15 — overall curriculum progress in the TUI
The progress bar showed only the current topic. It now also shows overall
progress across all exercises.
format_progress(...)inwidgets/progress.py;TrackScreen._render_statepasses overall counts (
len(state.completed)vslen(manifest.exercises)).#16 — celebrate finishing the whole curriculum
A per-topic message existed, but finishing everything had no payoff. Completing
the final pending exercise now shows a celebration (count, shareable line,
GitHub-star nudge). Gated on
next_pending(manifest.exercises, completed) is Noneso it fires only at true 100%; non-blocking.
Tests
TDD throughout — 7 new tests (
ensure_workspacex3, headless CLI auto-init x1,format_progressx2,celebration_messagex1). Full suite 125 → 132 passed,verified on Python 3.9 and 3.13.
Summary by CodeRabbit